fix(android): drop native-tls/openssl-sys from Android build; bump setup-android to v4#1336
Conversation
The Android release build failed to compile openssl-sys for
aarch64-linux-android ("Could not find directory of OpenSSL
installation"). openssl-sys is pulled in solely by reqwest's native-tls
feature, which backs the desktop mTLS PKCS#12 client-certificate path
(issue #1220) and has no use on Android; openssl-sys has no vendored NDK
cross build.
Move native-tls to a cfg(not(target_os = "android")) target dependency
and cfg-gate the code that uses it (the ClientIdentity::Pkcs12 variant
and the native-tls builder path). Android surfaces a clear error if a
PKCS#12 cert is configured rather than misreading it as PEM. Desktop
builds are unchanged (openssl-sys stays in their dependency tree).
Also bump android-actions/setup-android v3 -> v4.0.1 so the step runs on
Node.js 24 (v3 targeted the now-deprecated Node.js 20).
✅ Deploy Preview for geolibre-app ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughAndroid CI updates its setup action. Desktop Rust dependencies enable ChangesAndroid TLS compatibility
Estimated code review effort: 2 (Simple) | ~10 minutes Possibly related PRs
Suggested reviewers: Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
⚡ Cloudflare Pages preview
|
| Err(format!( | ||
| "PKCS#12 client certificates are not supported on this platform: {}", | ||
| path.display() | ||
| )) |
There was a problem hiding this comment.
Nice, clear failure mode instead of silently misreading the bundle. One follow-up: this is a genuinely new platform limitation (PKCS#12 client certs work on desktop, not on Android), but neither docs/android.md's "Known limitations / follow-ups" section nor the mTLS env-var table in docs/architecture.md (lines 120-129) mentions it. Worth a one-line addition to docs/android.md so this doesn't surprise someone configuring GEOLIBRE_HTTP_CLIENT_CERT on an Android build later.
Confidence: medium.
Code reviewI reviewed the diff ( Bugs
Security
Performance
Quality
CLAUDE.md
|
Problem
The v2.2.0 Android release build (run 29668727337) failed while cross-compiling
openssl-sysforaarch64-linux-android:openssl-sysis pulled in only byreqwest'snative-tlsfeature, which was added for the desktop mutual-TLS PKCS#12 client-certificate path (#1220). Android has no such path, andopenssl-syshas no vendored NDK cross build — so the whole toolchain has nothing to link against.The run also emitted a deprecation warning:
android-actions/setup-android@…(v3) targets the now-removed Node.js 20.Fix
native-tlsoff Android. Moved thenative-tlsfeature from the sharedreqwestdependency to acfg(not(target_os = "android"))target dependency, andcfg-gated the code that uses it (ClientIdentity::Pkcs12+ theuse_native_tls()builder arm). Android now returns a clear "PKCS#12 client certificates are not supported on this platform" error instead of silently misreading the bundle as PEM. Desktop builds are unchanged —openssl-sysstays in their dependency tree.android-actions/setup-androidv3 → v4.0.1, which runs on Node.js 24.Verification
cargo tree -i openssl-sysfor--target aarch64-linux-android→ nothing to print (removed); host target still shows it vianative-tls → reqwest.cargo check --liband theclient_certunit tests pass on the host.Summary by CodeRabbit